snakefmt/blocken.py (7)

1859-1866: Parameter input shadows Python builtin.

Consider renaming to source or content to avoid shadowing the builtin.

📝 Suggested fix

-def parse(input: str | Callable[[], str], name: str = "<string>"):
-    if isinstance(input, str):
+def parse(source: str | Callable[[], str], name: str = "<string>"):
+    if isinstance(source, str):
         tokens = tokenize.generate_tokens(
-            iter(input.splitlines(keepends=True)).__next__
+            iter(source.splitlines(keepends=True)).__next__
         )
     else:
-        tokens = tokenize.generate_tokens(input)
+        tokens = tokenize.generate_tokens(source)
     return GlobalBlock(0, TokenIterator(name, tokens), [])

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@snakefmt/blocken.py` around lines 1859 - 1866, The parse function currently
names its parameter input which shadows the Python builtin; rename the parameter
to a non-shadowing name like source (or content) in the parse definition and
update all references inside the function (the isinstance check, the
tokenize.generate_tokens call and the TokenIterator invocation) to use the new
name, and then update any callers of parse to pass the renamed parameter
positionally or by the new name if keyword-used.

---

1086-1086: Typo in comment.

"olgorithm" should be "algorithm".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@snakefmt/blocken.py` at line 1086, Fix the typo in the inline comment that
reads "please do not blame if the olgorithm is slow :)" by changing "olgorithm"
to "algorithm" so the comment becomes "please do not blame if the algorithm is
slow :)"; locate the comment text in snakefmt/blocken.py and update it
accordingly.

---

57-65: Redundant assignment on line 61.

self.tokens = tokens appears redundant since self._live_tokens = tokens is already assigned on line 59. If self.tokens is intended for external access, consider documenting its purpose or removing if unused.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@snakefmt/blocken.py` around lines 57 - 65, The assignment self.tokens =
tokens in __init__ is redundant with self._live_tokens; remove the redundant
self.tokens assignment or, if external access is required, replace it with a
documented property that returns self._live_tokens (e.g., implement a tokens
`@property`) so all uses reference a single source of truth; update any external
usages of the attribute to use the chosen API (either the property name tokens
or the internal _live_tokens) and add a short docstring to clarify intended
external access.

---

1800-1802: Typo: "sort_direcives" should be "sort_directives".

The attribute name has a consistent typo throughout the codebase. While it works, fixing it would improve code readability.

The typo also appears at:

• Line 329 in FormatState
• Line 1821 in get_formatted
• Line 1881 in setup_formatter

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@snakefmt/blocken.py` around lines 1800 - 1802, Rename the misspelled
attribute sort_direcives to sort_directives everywhere to improve readability:
update the __slots__ tuple ("mode", "sort_direcives") to ("mode",
"sort_directives"), rename the attribute annotations (mode: Mode;
sort_direcives: bool -> sort_directives: bool) and change all
references/assignments/usages in FormatState, get_formatted, and setup_formatter
to use sort_directives; ensure any constructors, attribute accesses, and tests
are updated to the new name and run linters/tests to catch any remaining
references.

---

1833-1842: Typo: "place_hode" should be "placeholder".

The variable names place_hode_str and place_hode appear to be typos for "placeholder".

📝 Suggested fix

-        place_hode_str = "o" * 50
+        placeholder_str = "o" * 50
         raw_str = "".join(python_codes)
-        while place_hode_str in raw_str:
-            place_hode_str *= 2
+        while placeholder_str in raw_str:
+            placeholder_str *= 2
         raw_str = "#\n"
         for python_code, (snakemake_code, indent) in zip(python_codes, snakemake_codes):
             if snakemake_code.count("\n") == 1:  # must at the end of line
-                place_hode = f"{indent}def l{place_hode_str}1ng(): ...\n"
+                placeholder = f"{indent}def l{placeholder_str}1ng(): ...\n"
             else:
-                place_hode = f"{indent}def l{place_hode_str}ng():\n{indent} return\n"
-            raw_str += python_code + place_hode
+                placeholder = f"{indent}def l{placeholder_str}ng():\n{indent} return\n"
+            raw_str += python_code + placeholder

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@snakefmt/blocken.py` around lines 1833 - 1842, The variables place_hode_str
and place_hode in the block inside the function that builds raw_str (the loop
using python_codes and snakemake_codes) are typos and should be renamed to
placeholder_str and placeholder respectively; update all usages including the
initial assignment, the while loop that multiplies the string, and the f-strings
that reference them (e.g., f"{indent}def l{place_hode_str}1ng(): ..." and
f"{indent}def l{place_hode_str}ng():") so they become f"{indent}def
l{placeholder_str}1ng(): ..." and f"{indent}def l{placeholder_str}ng():", and
ensure you rename the temporary variable place_hode to placeholder where it’s
assigned and used.

---

96-99: Minor typos in docstring.

"Donot" should be "Do not" and "INDEDT" should be "INDENT".

📝 Suggested fix

     def next_block(self):
         """Returns a entire block, just consume until the end of the block.
-        Donot care if there are nested blocks inside or snakemake keywords inside.
+        Do not care if there are nested blocks inside or snakemake keywords inside.

-        it could be INDEDT -> [any content] -> DEDENT, or [any content] -> DEDENT
+        it could be INDENT -> [any content] -> DEDENT, or [any content] -> DEDENT
         """

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@snakefmt/blocken.py` around lines 96 - 99, Fix the typos in the docstring in
snakefmt/blocken.py that documents the routine which "Returns a entire block"
(the docstring describing consuming until the end of a block); change "Donot" to
"Do not" and "INDEDT" to "INDENT", and while here correct "a entire block" to
"an entire block" for grammar consistency so the docstring reads clearly.

---

443-448: Error message note may be misleading when line number is available.

The "line number may be incorrect" note is always appended, even when start_token is provided and the line number is adjusted. Consider only adding this note when the line number couldn't be determined.

📝 Suggested fix

         else:
             err_msg = str(e)
-        err_msg += (
-            "\n\n(Note reported line number may be incorrect, as"
-            " snakefmt could not determine the true line number)"
-        )
+        if start_token is None or not match:
+            err_msg += (
+                "\n\n(Note reported line number may be incorrect, as"
+                " snakefmt could not determine the true line number)"
+            )
         err_msg = f"Black error:\n```\n{err_msg}\n```\n"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@snakefmt/blocken.py` around lines 443 - 448, The error text always appends
the ambiguous line-number note before wrapping and raising InvalidPython; change
the logic in the block that builds err_msg (the code that presently appends
"\n\n(Note reported line number may be incorrect...)" and then wraps err_msg and
raises InvalidPython) to only add that ambiguous-line-number note when the
start_token (or whatever marker is used to compute an adjusted line number) is
not available or the line could not be determined; keep the existing formatting
that wraps err_msg in "Black error:\n```\n...\n```\n" and then raise
InvalidPython(err_msg) from None but gate the extra note on start_token being
falsy/None (or on the failure case that determines line-number uncertainty).

tests/test_blocken.py (1)

29-32: Consider renaming parameter to avoid shadowing builtin.

The parameter input shadows the Python builtin. While harmless in this limited scope, using source or code would be cleaner.

📝 Suggested fix

-def generate_tokens(input: str):
+def generate_tokens(source: str):
     return list(
-        tokenize.generate_tokens(iter(input.splitlines(keepends=True)).__next__)
+        tokenize.generate_tokens(iter(source.splitlines(keepends=True)).__next__)
     )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_blocken.py` around lines 29 - 32, The parameter name `input` in
generate_tokens shadows the Python builtin; rename it (e.g., to `source` or
`code`) in the function signature and update all references inside the function
(the call to input.splitlines and any callers in tests) to use the new name so
the tokenizer still receives the same string argument.

tests/test_formatter.py (1)

2444-2444: Unused variable formatted0.

The unpacked variable formatted0 is never used in this test. Use underscore prefix to indicate it's intentionally ignored.

📝 Suggested fix

-        code1, formatted0 = TestSortFormatting.sorting_comprehensive
+        code1, _formatted0 = TestSortFormatting.sorting_comprehensive

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_formatter.py` at line 2444, The test unpacks
TestSortFormatting.sorting_comprehensive into code1, formatted0 but never uses
formatted0; change the unused variable to a deliberately ignored name (e.g.,
_formatted0 or _) in the assignment where
TestSortFormatting.sorting_comprehensive is unpacked so the intent is clear and
linters won't flag formatted0 as unused.

.flake8 (1)

3-4: Comment is now outdated.

The comment says "the default ignores minus E704", but E704 is now explicitly added to the ignore list. Consider updating the comment to reflect the current state.

📝 Suggested fix

-# the default ignores minus E704
-ignore = E121,E123,E126,E226,E203,E24,E701,E704,W503,W504
+# the default ignores plus E701 and E704 for single-line class/def patterns
+ignore = E121,E123,E126,E226,E203,E24,E701,E704,W503,W504

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.flake8 around lines 3 - 4, The comment on the .flake8 file is stale: update
or remove the phrase "the default ignores minus E704" to reflect that E704 is
now explicitly included in the ignore list; edit the comment above the ignore =
E121,E123,E126,E226,E203,E24,E701,E704,W503,W504 line (or replace it) to
accurately describe the current ignore configuration or remove the misleading
text.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

snakefmt/blocken.py (5)

446-446: Redundant str() call.

err_msg is already a string at this point. The str() conversion is unnecessary:

-        err_msg = f"Black error:\n```\n{str(err_msg)}\n```\n"
+        err_msg = f"Black error:\n```\n{err_msg}\n```\n"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@snakefmt/blocken.py` at line 446, The f-string that builds the Black error
message unnecessarily calls str() on err_msg even though err_msg is already a
string; replace the f-string expression "{str(err_msg)}" with "{err_msg}" so the
line constructing err_msg (the variable err_msg used to format the Black error
block) uses the value directly without redundant conversion.

---

532-534: Mutable class attribute defaults flagged by static analysis.

Ruff warns about mutable default values for class attributes (RUF012). While these appear to be intentionally shared registries, consider using ClassVar type hints to make the intent explicit:

from typing import ClassVar

class Block(ABC):
    subautomata: ClassVar[Mapping[str, "type[ColonBlock]"]] = {}
    deprecated: ClassVar[Mapping[str, str]] = {}

This also applies to similar patterns at lines 908, 959-960, 1725, 1785, and 1804.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@snakefmt/blocken.py` around lines 532 - 534, The class-level mutable mappings
subautomata and deprecated should be annotated as ClassVar to indicate
intentional shared registries and silence RUF012; update their type hints to use
ClassVar (e.g., subautomata: ClassVar[Mapping[str, "type[ColonBlock]"]] and
deprecated: ClassVar[Mapping[str, str]]), add the necessary typing import if
missing, and apply the same change to the other occurrences referenced (around
the symbols at the other noted locations) so all mutable class defaults are
explicitly ClassVar-annotated.

---

360-360: Minor: Prefer next() over single-element slice.

Per Ruff RUF015, consider using next() for extracting the first element:

-            option = [opt.strip() for opt in (options or "").split(",")][0]
+            option = next(opt.strip() for opt in (options or "").split(","))

This is more idiomatic and avoids creating an intermediate list.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@snakefmt/blocken.py` at line 360, Replace the single-element list
comprehension used to set the variable option with an iterator + next() to avoid
building an intermediate list: change the assignment to use next((opt.strip()
for opt in (options or "").split(",")), "") so it yields the first stripped
element (or a safe default) — update the line that currently sets option =
[opt.strip() for opt in (options or "").split(",")][0] in snakefmt/blocken.py.

---

1833-1837: Confusing variable reuse - raw_str assigned then immediately overwritten.

The code assigns raw_str = "".join(python_codes) at line 1834, uses it for collision checking, then immediately overwrites it at line 1837 with raw_str = "#\n". While functionally correct (the collision check uses the first value), this is confusing:

-        placeholder = "o" * 50
-        raw_str = "".join(python_codes)
+        placeholder = "o" * 50
+        all_python = "".join(python_codes)
         while placeholder in raw_str:
             placeholder *= 2
-        raw_str = "#\n"
+        raw_str = "#\n"

Consider using a distinct variable name for clarity:

         placeholder = "o" * 50
-        raw_str = "".join(python_codes)
-        while placeholder in raw_str:
+        all_python = "".join(python_codes)
+        while placeholder in all_python:
             placeholder *= 2
         raw_str = "#\n"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@snakefmt/blocken.py` around lines 1833 - 1837, The variable raw_str is reused
confusingly: it’s set to "".join(python_codes) for the collision check then
immediately overwritten with "#\n"; rename the first usage to a distinct
variable (e.g., joined_python or python_blob) and use that in the while
placeholder in <joined_variable> loop, then keep raw_str = "#\n" as the intended
subsequent value; update references in the collision-checking loop (placeholder
doubling) to use the new joined variable and leave raw_str only for the "#\n"
assignment.

---

1838-1852: Add strict=True to zip() calls to catch length mismatches.

The zip() calls on lines 1838 and 1847 pair python_codes with snakemake_codes. If there's ever a bug in the iteration logic that causes these lists to have different lengths, data would be silently dropped:

-        for python_code, (snakemake_code, indent) in zip(python_codes, snakemake_codes):
+        for python_code, (snakemake_code, indent) in zip(python_codes, snakemake_codes, strict=True):

And:

-        for formatted, (snakemake_code, _) in zip(formatted_split, snakemake_codes):
+        for formatted, (snakemake_code, _) in zip(formatted_split, snakemake_codes, strict=True):

This will raise ValueError if lists have different lengths, making bugs easier to detect.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@snakefmt/blocken.py` around lines 1838 - 1852, The two zip() usages pairing
python_codes and snakemake_codes should be made strict to surface length
mismatches: update the first loop that iterates "for python_code,
(snakemake_code, indent) in zip(python_codes, snakemake_codes):" and the second
loop "for formatted, (snakemake_code, _) in zip(formatted_split,
snakemake_codes):" to use zip(..., strict=True). This ensures mismatched lengths
raise ValueError during the preparation of raw_str/py proxies and during
reconstruction of final_str after format_black, helping catch bugs early.

tests/test_formatter.py (1)

2396-2401: # fmt: skip in directives produces unusual output.

The expected output shows x = [ 1,2,3] # fmt: skip followed by a trailing comma on its own line (TAB * 2 + ",\n"). This is noted in the comment as a limitation ("currently # fmt: skip in directives is not supported").

Consider whether this behavior should be:

1. Fixed to properly handle # fmt: skip inline
2. Documented as a known limitation in user-facing docs
3. Tracked as a future enhancement issue

Would you like me to open an issue to track improving # fmt: skip support in directives?

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_formatter.py` around lines 2396 - 2401, The test shows a directive
line containing "# fmt: skip" (seen in the test case using
formatter.get_formatted()) but the formatter leaves a trailing comma on the next
line; update the directive-handling logic (e.g., the Formatter method that
processes directives — look for functions named like process_directives,
_apply_directives, or _should_skip_range) to treat an inline "# fmt: skip" as
skipping the entire directive token sequence (including following separator
tokens such as trailing commas) so no lone comma line is emitted; adjust the
formatter's token-range skip logic to remove or absorb following punctuation
when a directive applies, add a focused unit test mirroring this example, and if
you prefer not to change behavior add/update docs/tests to explicitly record
this limitation instead.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes